Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Lang] Migrate irpass::scalarize() after irpass::demote_operations() #8096

Merged
merged 13 commits into from
May 31, 2023

Conversation

jim19930609
Copy link
Contributor

@jim19930609 jim19930609 commented May 29, 2023

Issue: #

Brief Summary

🤖 Generated by Copilot at e852b32

This pull request fixes some bugs and improves some optimizations in the LLVM code generation and the IR transformation passes. It introduces a new type TensorType and handles it correctly in the demote_operations and codegen_llvm passes. It also refines the scalarization and block-local optimization of the IR nodes, by applying them selectively, conditionally, and with proper simplification.

Walkthrough

🤖 Generated by Copilot at e852b32

  • Fix a bug in codegen_llvm.cpp where the signedness of the return type of a binary division or bit shift right operation was not checked correctly (link, link)
  • Modify the scalarize function in ir/transforms.h and scalarize.cpp to return a bool value indicating whether the IR node has been modified or not (link, link, link, link)
  • Remove the redundant and unnecessary calls to the delayed_modifier_.modify_ir() method in scalarize.cpp, which are replaced by the bool return values of the scalarize passes (link, link, link)
  • Add the conditional call to the scalarize function in compile_to_offloads.cpp, where it is applied to each offload node or function node that has the block_local flag set, and perform the full simplification or dead instruction elimination if the node has been modified by the scalarize function (link, link)
  • Remove the call to the scalarize function in compile_to_offloads.cpp, where it was applied to the whole IR node before the offload_to_executable function, which is unnecessary and could affect the performance or correctness of the code generation (link)
  • Add some code to the make_block_local.cpp file, where it checks whether the offload node accesses any block-local SNodes, and applies the scalarize pass and the full simplification if so (link)
  • Remove some code from the demote_operations.cpp file, where it handled the case of TensorTypes for power operations or both operands, which is unnecessary and incorrect because the scalarize pass already handles the case of TensorTypes (link, link)
  • Add some code to the demote_operations.cpp file, where it handles the case of TensorTypes for the left-hand side of a binary operation, by creating a zero value of the same TensorType and replacing the usage of the original left-hand side with the zero value (link)
  • Modify the demote_operations.cpp file, where it handles the case of TensorTypes for floor division or bit shift right operations, by using the get_element_type() method to get the primitive type of the operands and passing them to the is_integral(), is_signed(), or is_real() functions (link, link, link, link, link, link)
  • Modify the static run method of the MergeExternalAndMatrixPtr class in scalarize.cpp to return a bool value indicating whether the IR node has been modified or not (link)

@netlify
Copy link

netlify bot commented May 29, 2023

Deploy Preview for docsite-preview canceled.

Name Link
🔨 Latest commit d9bcabf
🔍 Latest deploy log https://app.netlify.com/sites/docsite-preview/deploys/6475ba73b3f9f10008985cda

Copy link
Contributor

@dream189free dream189free left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jim19930609 jim19930609 merged commit 67e9084 into taichi-dev:master May 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants